refactor(Label): cleanup docs and simplify render#642
Conversation
dvdzkwsk
requested review from
kuzhelov,
layershifter,
levithomason,
miroslavstastny and
mnajdova
as code owners
src/components/Label/Label.tsx
Outdated
There was a problem hiding this comment.
Factories already handle falsy values in the first position, so we don't need to handle that ourselves.
src/components/Label/Label.tsx
Outdated
There was a problem hiding this comment.
Remove reassignment of local variables. We can simply render elements inline in their target position.
src/components/Label/Label.tsx
Outdated
There was a problem hiding this comment.
Make terminology consistent. Allowed prop values are "start" and "end", so we should use the same wording in the description.
dvdzkwsk
force-pushed
the
refactor/label
branch
from
c2b3b04 to
d8434a9
Compare
dvdzkwsk
force-pushed
the
refactor/label
branch
from
d8434a9 to
8a51b36
Compare
kuzhelov
added
ready for merge
needs author changes
|
UTs need to pass before merging |
|
UTs are fixed now - there was a problem with filtering logic implemented as part of shorthand tests |
kuzhelov
removed
the
needs author changes
|
Thanks for the UT fix, @kuzhelov. |
…ardust-ui/react into refactor/label # Conflicts: # CHANGELOG.md
layershifter
left a comment
layershifter
left a comment
There was a problem hiding this comment.
I really like changes in this PR, superior work 👍
But, there is an issue that causes visual regressions. Layout will add a gap if start/end are present, after these changes they will be always present and it will produce invalid gaps 😭
We have already discussed this issue with @kuzhelov, we can make following changes:
const startImage = imagePosition === 'start' && imageElement
const starIcon = iconPosition === 'start' && iconElement
const hasStart = startImage || startIcon
<Layout start={hasStart && (
<>
{startImage}
{startIcon}
</>
)}It can be a solution, but I'm open for better ideas.
…ardust-ui/react into refactor/label # Conflicts: # CHANGELOG.md
…factor/label # Conflicts: # CHANGELOG.md
6 participants
This PR cleans up some inconsistencies in the doc strings, and simplifies the render method for readability. There are no functionality changes; this is simply a reduction of source code.